Skip to content

Correct the dynamic-framework claim and record the linking spike - #145

Merged
kyleve merged 3 commits into
mainfrom
record-dynamic-linking-findings
Jul 27, 2026
Merged

Correct the dynamic-framework claim and record the linking spike#145
kyleve merged 3 commits into
mainfrom
record-dynamic-linking-findings

Conversation

@kyleve

@kyleve kyleve commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Docs only. Corrects a load-bearing claim that turned out to be false, and files the dynamic-linking spike so it isn't lost.

The correction

The root AGENTS.md double-linking rule — and SnapshotKitTesting/AGENTS.md — justified themselves with "WhereUI is a dynamic framework that statically embeds its own dependencies."

The build disagrees. There are no .framework products anywhere, and every .app/Frameworks is empty. The local package is wired as an XCLocalSwiftPackageReference and handed to Xcode's own SPM integration, which links every product statically into each consumer. WhereUI is ~50,000 symbols compiled separately into each test bundle that links it.

The rule is unchanged and still correct — a double-link genuinely lands two copies in one image. Only its stated mechanism was wrong. That matters practically: the wrong mechanism is what made "just make it dynamic" look like a cheap fix, which is how this investigation started.

The section is renamed to match reality, and the four AGENTS.md files linking its anchor follow.

One measured nuance

It predicts which duplications actually bite:

Symbol kind Linkage Coalesced by dyld?
public type metadata (e.g. SnapshotCaptureTrait) external Yes — one definition wins
file-scope private global / enum static var (e.g. _swizzleDepth) non-external Never — one per image

Verified by splitting SnapshotKit across two images and watching \.isCapturingSnapshot keep working — which contradicted my own prediction that it would break. Same experiment explains why SnapshotKitTesting's capture state duplicates while the trait doesn't.

I've deliberately framed this as "the guard test is the authority on whether a given duplication is harmful" rather than as a licence to double-link. The historical WhereUI failure was real, cost someone two hours, and is still guarded by WhereStylesheetTests.resolvesTraitAwareTokensFromTheBroadwayRoot. One experiment isn't grounds to relitigate it.

The spike, filed as a P2 (not landed)

It works. One line — .library(name: "SnapshotKitTesting", type: .dynamic, …) — takes the .xctest's private _swizzleDepth count from one-per-bundle to zero, with the bundle linking @rpath/SnapshotKitTesting.framework. That would lift the "never add a second image-snapshot bundle" rule.

Why it's not landed. A dynamic product that statically absorbs a resource-bearing dependency orphans that dependency's resources. AccessibilitySnapshotParser's code moves into the framework while its .bundle stays in the .xctest; SwiftPM's generated accessor searches only Bundle.main.resourceURL, Bundle(for: BundleFinder.self).resourceURL, Bundle.main.bundleURL, so Bundle.module hits its fatalError and every VoiceOver-annotated capture traps. Co-locating the bundles with the framework fixes it (verified — accessibility suites pass, no pixel drift), but automating that needs a build phase against a framework Xcode's SPM integration generates. That's the same Bundle.module placement fragility the StuffTestHost WhereCore embed was, bought for a constraint that currently costs nothing.

The entry also records two dead ends and one trap, so a future attempt doesn't repeat them:

  • Tuist's PackageSettings(productTypes:) does nothing for a local package — only SwiftPM's type: works.
  • type: .dynamic applies only to products an Xcode target consumes as a product; WhereUI depends on the SnapshotKit target, so SnapshotKit stayed static despite the annotation.
  • A shared DerivedData reports false negatives here. Two runs reported "no frameworks produced" from an incremental build that hadn't re-resolved the package graph. Spike into a fresh -derivedDataPath or it will lie to you.

Verification

./swiftformat --lint clean; ./sync-agents run. No source or reference-image changes. All cross-file anchors updated and confirmed resolving.

The root AGENTS.md double-linking rule, and SnapshotKitTesting's AGENTS.md,
both justified themselves with "WhereUI is a dynamic framework that statically
embeds its own dependencies". The build disagrees: there are no .framework
products anywhere and every .app/Frameworks is empty. The local package is
wired as an XCLocalSwiftPackageReference and handed to Xcode's own SPM
integration, which links every product statically into each consumer.

The rule itself is unchanged and still right -- a double-link genuinely lands
two copies in one image. Only its stated mechanism was wrong, which matters
because the wrong mechanism is what made "just make it dynamic" look like an
easy fix. Renames the section accordingly (the four AGENTS.md files linking
its anchor follow) and states the actual topology.

Adds one measured nuance that predicts which duplications bite: external
symbols coalesce across images, non-external ones never do. A public type's
metadata is external, so dyld picks one definition and type-keyed lookups
still agree -- verified by splitting SnapshotKit across two images and
watching \.isCapturingSnapshot keep working, contradicting my own prediction
that it would break. A file-scope private global or an enum's static var is
non-external and genuinely per copy, which is exactly why
SnapshotKitTesting's capture state duplicates. Deliberately framed as "the
guard test is the authority", not as a licence to double-link: the historical
WhereUI failure was real and is still guarded.

Files the dynamic-linking spike as a P2 rather than leaving it in a chat log.
It works -- one line in Package.swift takes the .xctest's private
_swizzleDepth count to zero -- but it orphans the resource bundles of any
resource-bearing dependency the dynamic product statically absorbs, so every
VoiceOver-annotated capture hits Bundle.module's fatalError. Co-locating the
bundles with the framework fixes it (verified, no pixel drift), but
automating that reintroduces the Bundle.module placement fragility that the
StuffTestHost WhereCore embed was. Not landed. The entry records that, the
two dead ends (Tuist PackageSettings does not apply to a local package;
type: .dynamic only applies to products consumed as products), and the trap
that cost me two wrong conclusions: a shared DerivedData reports false
negatives, so spike it into a fresh -derivedDataPath.

Docs only; no behavior change.
Gives each module its own image-snapshot bundle — `WhereUISnapshotTests`, `PeriscopeToolsSnapshotTests`, `SwiftDataInspectorSnapshotTests` — all listed in the single `StuffSnapshotTests` scheme, so CI still runs them in one invocation.

**Stacked on #145.** Review/merge that first.

## This corrects my mistake in #143

I argued there for a single shared bundle, on the grounds that a second one would be unsafe: several `.xctest` bundles in one scheme load into one `StuffTestHost` process, where each bundle's own copy of `SnapshotKitTesting`'s capture state (the safe-area swizzle's depth counter, `SnapshotCaptureLock`, the `UIView` category) would fight over the single shared `UIView` method exchange.

**The premise is false.** I inherited "one scheme means one process" from the root `AGENTS.md` double-linking note instead of measuring it. Probing `ProcessInfo.processIdentifier` from two bundles in one scheme:

```
full unfiltered Stuff-iOS-Tests run:
  SnapshotKitTestingTests  pid=12076
  WhereUITests             pid=12146
```

Different PIDs, on both a filtered and a full run. xcodebuild gives each test bundle its own host process, so each copy of that state is genuinely process-wide — exactly as its documentation claims. The hazard is real, but it needs co-loading, which doesn't happen.

## Splitting is better than merely permissible

Each module's bundle now links only what that module needs. `PeriscopeToolsSnapshotTests` depends on `PeriscopeTools` and `SwiftDataInspectorSnapshotTests` on `SwiftDataInspector` — neither builds against WhereUI any more. That's the layering wart I explicitly accepted when arguing for the shared bundle, now gone.

References were already per-module (swift-snapshot-testing derives `__Snapshots__` from the caller's `#filePath`), so **no image moved and none were re-recorded**.

## Docs

The "never add a second image-snapshot bundle" rule is replaced with the measurement and the assumption it rests on: if a toolchain ever starts sharing one host process across bundles, re-measure before adding another. Updated across the root `AGENTS.md`, `SnapshotKitTesting`, and the three modules' `AGENTS.md`/`README.md`.

The #143 completed TODO keeps its wrong reasoning with a correction appended rather than being edited away. Two load-bearing claims in these docs have now turned out to be inherited rather than verified — the "WhereUI is a dynamic framework" one in #145, and this — and that pattern seems worth leaving visible.

## Note on #145

Its filed spike item existed to make a second snapshot bundle safe. That motivation is now moot, so this PR reframes the entry: the findings stay recorded (they're still accurate about *how* dynamic linking behaves), but it's explicit that the reason for wanting it evaporated.

## Verification

- All three bundles green from the one scheme, with three separate `All tests` boundaries in the output — the process separation showing up directly.
- **Zero reference images modified.**
- `Stuff-iOS-Tests` green.
- `./swiftformat --lint` clean.
@kyleve

kyleve commented Jul 27, 2026

Copy link
Copy Markdown
Owner Author

@cursoragent Please merge main into this PR. Once merged and pushed, no need to monitor.

@cursor

cursor Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Merged origin/main into record-dynamic-linking-findings and pushed. One real conflict, in TODOs.md: main's new feat(PeriscopeCore) P2 landed at the same spot as this branch's refactor(SnapshotKitTesting) spike entry, so I kept both (the spike entry with its four sub-bullets first, then the PeriscopeCore item, then the existing StuffTestHost one). Everything else — AGENTS.md, Project.swift, Package.swift, .github/workflows/ci.yml, Where/AGENTS.md, Where/WhereUI/README.md — auto-merged.

Two things I checked rather than assumed, since main brought in the Settings About page and CreditKit:

  • main added AboutSettingsViewSnapshotTests.swift / LicenseViewSnapshotTests.swift under Where/WhereUI/SnapshotTests/, which this branch's per-module split still globs into WhereUISnapshotTests (sources: ["Where/WhereUI/SnapshotTests/**"]), so the new suites are picked up and nothing is orphaned. Their only extra import is SnapshotKitTesting, which that target already lists.
  • This PR's anchor rename (#never-double-link-a-product-whereui-already-carries) still resolves from all four linking files, and none of main's new docs reference the old anchor.

Verification on this Linux VM: swiftformat --lint clean (0/658 files need formatting), ./attribution --check reports attribution.json: up to date (7 credit(s)) — worth running because that gate is new from main — and ./sync-agents regenerated the gitignored CLAUDE.md mirrors including CreditKit's. ./xcstrings --lint can't run here (it's a #!/usr/bin/swift script and there's no Swift toolchain on the VM); no .xcstrings was touched by the conflict resolution. Not monitoring CI, as you asked.

Open in Web Open in Cursor 

…g-findings

# Conflicts:
#	TODOs.md

Co-authored-by: Kyle Van Essen <[email protected]>
@kyleve
kyleve merged commit 09100c1 into main Jul 27, 2026
4 checks passed
cursor Bot pushed a commit that referenced this pull request Jul 27, 2026
PR #145 established what makes the removed WhereCore dependency
unnecessary: nothing here is a dynamic framework, and Xcode's SPM
integration links every product statically into each consumer, which is
why a `.xctest` carries its own resource bundles. Point the host's note
at that rule rather than asserting the consequence on its own.

Also refresh the scene-name item's line numbers, which the merges since
have moved.

Co-authored-by: Kyle Van Essen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants